Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add tests for authorizers #846

Merged
merged 17 commits into from
Oct 28, 2023
Merged

Add tests for authorizers #846

merged 17 commits into from
Oct 28, 2023

Conversation

peterlimg
Copy link
Member

@peterlimg peterlimg commented Aug 29, 2023 •
edited
Loading

  • Add tests to register and delete authorizer from DEX SC
  • Add tests to register and delete authorizer from zcnsc
  • Add tests to replace authorizers and test the burn-zcn mint-wzcn before and after

Related PRs:

@peterlimg peterlimg mentioned this pull request Sep 3, 2023
Merged
3 tasks
@peterlimg peterlimg changed the base branch from master to sprint-1.11 September 4, 2023 05:18
YarikRevich
YarikRevich requested changes Oct 10, 2023
View reviewed changes
tests/cli_tests/zwalletcli_zcnbridge_auth_replace_burn_mint_test.go Show resolved Hide resolved
tests/cli_tests/zwalletcli_zcnbridge_auth_replace_burn_mint_test.go Outdated
require.NoError(t, err, "Unexpected remove authorizer failure", strings.Join(output, "\n"))
t.Logf("remove authorizer: %s zcnsc successfully", auth2.ID)

// add back the first removed authorizer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case according to smart contracts logic, the first authorizer will be added again, not the one with index 0. It does not affect actual logic here, but this comments may be confusing.

Copy link
Member Author

@peterlimg peterlimg Oct 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what the index 0 do you mean here, but what I'm intend to say is the index 0 of auths defined above. Anyway, rephrased the comment, hope it is clearer now.

tests/cli_tests/zwalletcli_zcnbridge_add_and_rm_authorizer_test.go Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can associate these test cases into one file regarding authorizer tests

@Kishan-Dhakan
Copy link
Contributor

Passing run https://github.com/0chain/system_test/actions/runs/6677271899/job/18147117920

@Kishan-Dhakan Kishan-Dhakan merged commit 70a4047 into sprint-1.11 Oct 28, 2023
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YarikRevich YarikRevich YarikRevich requested changes

@Kishan-Dhakan Kishan-Dhakan Kishan-Dhakan approved these changes

@stewartie4 stewartie4 Awaiting requested review from stewartie4

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@peterlimg @Kishan-Dhakan @YarikRevich @stewartie4